Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes attribute deserialization when using WebComponents | Fixes #292 #373

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

matuzalemsteles
Copy link
Contributor

@matuzalemsteles matuzalemsteles commented Apr 25, 2018

I've been looking at issue #292 and I've been noticing that the big problem of using boolean when it is false. In return of the function deserializeValue_ we have OR and as retVal gets false the return is value parameter that contains 'false'.

Note

There are some use cases that want to pass a number value to string, we can force it into the element attribute by passing "'10'", but the deserializeValue_ function receives ''10'' with two single quotes and breaks the JSON.parse, but the correct way would be to pass '"10"' to do the parser correctly.

Please feel free to offer a solution to avoid this.

Current behavior

screen shot 2018-04-25 at 18 28 29

Expected behavior

screen shot 2018-04-25 at 18 28 59

…emove the OR in return | Fixes metal#292

NOTE: It is not recommended to use the OR in this case once the deserilization of a boolean false is made, for example, the OR is false and return the value parameter in this case.
@matuzalemsteles
Copy link
Contributor Author

cc @jbalsas

@diegonvs
Copy link
Contributor

depends on #377

@yuchi
Copy link
Contributor

yuchi commented May 17, 2018

@matuzalemsteles what’s that repl-ish thing you used to make those screens?

@matuzalemsteles
Copy link
Contributor Author

hey @yuchi, I was using Scratches.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants